Skip to content

Conversation

@fabianfett
Copy link
Contributor

@fabianfett fabianfett commented May 20, 2020

Motivation

We want to be able to mock different aws service behaviors with the Lambda.LocalServer.

Changes

  • The Lambda.LocalServer now opens two ports: One for incoming requests (InvokeHandler) and one for the control plane API (ControlPlaneHandler)
  • The ServerState (please hand me ideas for better names) encapsulates all the "business" logic: Queueing incoming requests, ensuring control plane state, ...
  • The user can implement the LocalLambdaInvocationProxy protocol to mock the behavior of different aws services
  • a default InvokeProxy is included
  • The server has been moved to the AWSLambdaTesting target, since we want to be able to access AWSLambdaEvents, if we implement an APIGatewayV2Proxy

@fabianfett fabianfett force-pushed the feature/ff-local-invoke-configurable branch 2 times, most recently from bdadc76 to 67e3f3f Compare May 20, 2020 19:56
@fabianfett fabianfett force-pushed the feature/ff-local-invoke-configurable branch from 67e3f3f to cb80b11 Compare May 20, 2020 19:58
@fabianfett fabianfett requested a review from tomerd May 20, 2020 20:06
@tomerd tomerd mentioned this pull request May 21, 2020
@pokryfka
Copy link
Contributor

pokryfka commented Jul 2, 2020

what is the status of this PR? It would be useful to have it...

@fabianfett
Copy link
Contributor Author

@pokryfka the work on this pr has stopped, since we chose to go a different route with configuring the LocalServer. Since the LocalServer is now automatically started if the ENV variable is set, we need to find a different way. One idea is to provide a bootstrap method like in swift-logging to configure the LocalServer before actual startup. Does this make sense?

@pokryfka
Copy link
Contributor

pokryfka commented Jul 2, 2020

@pokryfka the work on this pr has stopped, since we chose to go a different route with configuring the LocalServer. Since the LocalServer is now automatically started if the ENV variable is set, we need to find a different way. One idea is to provide a bootstrap method like in swift-logging to configure the LocalServer before actual startup. Does this make sense?

So the bootstrap function (static method) and LocalLambdaInvocationProxy protocol would be public and remain in AWSLambdaRuntimeCore, something like:

public protocol LocalLambdaInvocationProxy { 
 // ...
}

public enum LocalLambda {
    public static func bootstrap(_ proxyType: LocalLambdaInvocationProxy.Type) {
      // ...
    }
}

Now APIGatewayV2Proxy depends on AWSLambdaEvents and needs JSON encoder and decoder. It would be defined in AWSLambdaTesting.

Then if user wants to mock API Gateway he would need to import AWSLambdaTesting (?):

#if DEBUG
import AWSLambdaTesting

LocalLambda.bootstrap(APIGatewayV2Proxy.self)
#endif

// this will run LocalLambda.Server if DEBUG and LOCAL_LAMBDA_SERVER_ENABLED is "true"
Lambda.run(APIGatewayProxyLambda())

@fabianfett is that what you had in mind?

on top of that, it would probably be convenient to have a few proxies (API Gateway V1 and V2) defined (in AWSLambdaRuntime?) and configurable using env variable:

 LOCAL_LAMBDA_SERVER_PROXY_TYPE=invoke // or api, api2

@pokryfka
Copy link
Contributor

pokryfka commented Jul 3, 2020

I just noticed MockLambdaServer and HTTPHandler in LambdaRuntimeClientTest look very much the same as the LocalLambda:Server and LocalLambdaInvocationProxy is almost the same as LambdaServerBehavior (with one using NIO future and the other Result).

Perhaps one more advantage of the approach in the PR could be (a possibility of) an implementation of a MockProxy used in tests instead of MockLambdaServer which in turn would make the local lambda server better tested.

public protocol LocalLambdaInvocationProxy {
     init(eventLoop: EventLoop)

      /// throws HTTPError
     func invocation(from request: HTTPRequest) -> EventLoopFuture<ByteBuffer>
     func processResult(_ result: ByteBuffer?) -> EventLoopFuture<HTTPResponse>
     func processError(_ error: ByteBuffer?) -> EventLoopFuture<HTTPResponse>
 }
 internal protocol LambdaServerBehavior {
     func getInvocation() -> GetInvocationResult
     func processResponse(requestId: String, response: String?) -> Result<Void, ProcessResponseError>
     func processError(requestId: String, error: ErrorResponse) -> Result<Void, ProcessErrorError>
     func processInitError(error: ErrorResponse) -> Result<Void, ProcessErrorError>
 }

@tomerd
Copy link
Contributor

tomerd commented Jul 6, 2020

Perhaps one more advantage of the approach in the PR could be (a possibility of) an implementation of a MockProxy used in tests instead of MockLambdaServer which in turn would make the local lambda server better tested.

I really like this approach. imo the priority is:

  1. come up with a good system to inject middleware for LocalLambda:Server to more easily test scenarios like API Gateway.
  2. refactor the testing the facilities to use LocalLambda:Server instead of MockLambdaServer (and remove MockLambdaServerif possible)

@pokryfka @fabianfett wdyt?

@pokryfka
Copy link
Contributor

pokryfka commented Jul 8, 2020

@fabianfett what are your thoughts on that?

I could help with that and:

  • sync the branch with master,
  • create static LocalLambda.bootstrap function as described above
  • implement APIGatewayProxyLambda using APIGatewayMapping from WIP: Mock API Gateway response #138

@fabianfett
Copy link
Contributor Author

fabianfett commented Jul 8, 2020

@pokryfka I'd be excited if you could help out here. Go ahead! Maybe we should try to split the changes in small prs so we don't end up with such a big one like the one we have here.

pokryfka pushed a commit to pokryfka/swift-aws-lambda-runtime that referenced this pull request Jul 16, 2020
@pokryfka
Copy link
Contributor

@fabianfett @tomerd please check the change on #138
(I dont think I could push to feature/ff-local-invoke-configurable and It would probably not make sense to make a PR to a branch which is not in sync with master?)

@tomerd
Copy link
Contributor

tomerd commented Oct 5, 2020

@fabianfett could you please update this PR to be against the main branch so we can delete the old master branch

@fabianfett fabianfett changed the base branch from master to main October 5, 2020 18:10
@tomerd
Copy link
Contributor

tomerd commented Jan 22, 2024

closing in inactive PRs, feel free to re-open if still relevant

@tomerd tomerd closed this Jan 22, 2024
@sebsto sebsto deleted the feature/ff-local-invoke-configurable branch October 8, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants